Infer types in Prometheus remote_write #19944
Conversation
botelastic
bot
added
the
needs_team
Signed-off-by: chrismark <[email protected]>
ChrsMark
force-pushed
the
infer_types_remote_write
branch
from
1d4c919 to
c0c6a0f
Compare
botelastic
bot
removed
the
needs_team
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
ChrsMark
added
[zube]: (7.10) Planned for release
v7.10.0
and removed
[zube]: (7.10) Planned for release
labels
|
Pinging @elastic/integrations-platforms (Team:Platforms) |
|
This one is ready for a first round of review. Implementation is complete along with unit tests to cover all the metrics processing chain from the heuristic approach of extracting types to grouping metrics with same labesets together. |
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
x-pack/metricbeat/module/prometheus/remote_write/_meta/fields.yml
Outdated
Show resolved
Hide resolved
Signed-off-by: chrismark <[email protected]>
ChrsMark
added
test-plan
Signed-off-by: chrismark <[email protected]>
|
jenkins run the tests please |
Signed-off-by: chrismark <[email protected]>
|
@exekias I did some cleanups and it looks like this is ready for a final review so feel free to hit it once you get the chance. |
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
| . `_count` suffix: the metric is of Counter type | ||
| . `_bucket` suffix and `le` in labels: the metric is of Histogram type | ||
|
|
||
| Everything else is handled as a Gauge. |
There was a problem hiding this comment.
Just curious, is there a reason to default to gauge and not to counter?
There was a problem hiding this comment.
Hmm, we check if a metric is a counter or a histogram and if not then we conclude that is a gauge. This is the "heuristic". If we fall-back to counters then we will not have a way to identify gauges.
There was a problem hiding this comment.
Yeah, I meant if there was a reason to add a heuristic to detect counters and handle everything else as gauges and not the other way around. Maybe it is more difficult to find a heuristic to detect gauges 🤔
Signed-off-by: chrismark <[email protected]>
| . `_count` suffix: the metric is of Counter type | ||
| . `_bucket` suffix and `le` in labels: the metric is of Histogram type | ||
|
|
||
| Everything else is handled as a Gauge. |
There was a problem hiding this comment.
Yeah, I meant if there was a reason to add a heuristic to detect counters and handle everything else as gauges and not the other way around. Maybe it is more difficult to find a heuristic to detect gauges 🤔
| const ( | ||
| counterType = "counter_type" | ||
| histogramType = "histgram_type" | ||
| otherType = "other_type" |
There was a problem hiding this comment.
Oh ok, it is not only about gauges 👍 Should we mention in some place that summaries are handled as gauges?
Signed-off-by: chrismark <[email protected]>
Signed-off-by: chrismark <[email protected]>
|
Thanks for the review @jsoriano! Regarding #19944 (comment), it is easier to identify counters and histograms, gauge does not follow a specific pattern. This approach was also defined at #19944 (comment). I also added a note in the docs about Summary handling 🍻 . |
(cherry picked from commit b797a7e)
…ne-2.0 * upstream/master: (29 commits) Add an explicit system test for processes on unix systems (elastic#20320) [Autodiscovery] Ignore ErrInputNotFinished errors in autodiscover config checks (elastic#20305) [CI] Update README.md with CI references (elastic#20316) Add ECK doc links to Heartbeat docs (elastic#20284) [Filebeat] Add export tests to x-pack/filebeat (elastic#20156) feat(ci): support building docker images for PRs (elastic#20323) Update system tests dependency (elastic#20287) [Libbeat] Log debug message if the Kibana dashboard can not be imported from the archive (elastic#12211) (elastic#20150) [Filebeat][Gsuite] Transform all dates to timestamp with processor (elastic#20308) Infer types in Prometheus remote_write (elastic#19944) Remove unnecessary restarts of metricsets while using Node autodiscover (elastic#19974) docs: update changelog on master branch (elastic#20259) feat(ci): support storing artifacts for PRs in separate dirs (elastic#20282) [CI] Change upstream reference (elastic#20296) [Filebeat] Updates to Suricata module (elastic#20220) [docs] Fix Windows download link for agent (elastic#20258) [docs] Rename release highlights to what's new (elastic#20255) fix: update the display name of the multibranch job (elastic#20265) [Elastic Agent] Add basic protocol to control Elastic Agent. (elastic#20146) Cisco ASA: Fix message 106100 (elastic#20245) ...
This PR add types for Prometheus metrics that are collected via
remote_writemetricset.These metrics are in raw format when received so a heuristic approach is applied to extract types from metrics' name patterns.
Closes #17675.
TODOs
How to test this PR
Env setup
Use https://github.com/ChrsMark/docker-prometheus-playground for a ready to use prometheus env.
For remote write this config is needed: https://github.com/ChrsMark/docker-prometheus-playground/blob/master/prometheus/prometheus.yml#L10
If using a
darwinmachine the following should be enough:On
linuxmachines in order to access host from within the Prometheus container one can setnetwork_mode: hostin thedocker-compose.ymland then host can be reach using0.0.0.0.Testing
collectorandremote_write(withouttypes) metricsets for regressions.collector(withtypes) for regressions.remote_writemetricset:a. uses types as expected.
b. can identify types based on input patterns: https://github.com/elastic/beats/pull/19944/files?file-filters%5B%5D=.asciidoc#diff-b69e84fad87eb2ae563115f43675a547R118